fix(auth): normalize bearer token whitespace for rate limit stability - #1291

Thanks for taking this on. The PR is necessary: it addresses the explicitly assigned Bearer-token whitespace hardening item from #337, and the core token.strip(); return token or None change correctly makes FastAPI-auth-equivalent Bearer values map to the same rate-limit identity.

I do not think the current revision is merge-ready yet:

1. Blocking — runtime annotation regression. Moving BaseEngine behind TYPE_CHECKING in vllm_mlx/config/server_config.py breaks runtime annotation resolution. On this branch, typing.get_type_hints(ServerConfig) raises NameError: name 'BaseEngine' is not defined; the same call succeeds on current main. This change is unrelated to the auth fix and should be removed (or redesigned in a separate PR with a runtime-resolvable annotation).

2. Blocking — overly broad test skips. The three new except ImportError: pytest.skip(...) blocks can turn a broken or partially installed MLX package, including a missing transitive dependency, into a green skip. That conceals packaging regressions in existing exception-handler tests. These changes are also outside the Bearer-normalization scope and should be removed. If cross-platform availability needs separate work, it should use a narrow availability check in a dedicated PR.

3. Test gap — pin the security contract, not only the helper. The new helper assertions are useful, but the regression being fixed is rate-limit bucket fragmentation. Please add/replace with a contract test using _rate_limit_client_id(Request(...)) and assert that these auth-equivalent values produce the same client ID:

   • Bearer test-secret
   • Bearer test-secret
   • Bearer test-secret
   • Bearer \t test-secret \n

   I verified locally that the proposed production change satisfies this bucket-level contract.

4. The branch is currently 14 commits behind main and has no reported GitHub checks. Please rebase after narrowing the diff and rerun the normal validation.

5. pr_validate also rejects the unchecked Updated README/docs if applicable checklist item. If it is not applicable, remove it or mark it explicitly complete/N/A.

Suggested minimal scope: vllm_mlx/middleware/auth.py plus the focused rate-limit bucket regression test. With the unrelated config/test-skip changes removed, the core fix looks sound.

Maintainer update — took this to merge-ready via maintainerCanModify (thanks @2005rishabh, your commit authorship is preserved).

Rebased onto current main (the branch was ~pre-audio/video-refactor and diffed noisily) and narrowed to exactly the sound part:

• Kept: _extract_bearer_token → token = token.strip(); return token or None.
• Added: a bucket-level contract test (_rate_limit_client_id on Bearer test-secret vs whitespace-padded variants → same client id), alongside your helper edge-case test. This pins the actual security regression (bucket fragmentation), not just the helper.
• Dropped (out of scope, both flagged in review): the TYPE_CHECKING gate on server_config.py (it broke runtime typing.get_type_hints(ServerConfig) → NameError: BaseEngine), and the three except ImportError: pytest.skip guards (a broken/partial MLX install would silently green-skip and hide packaging regressions).

Diff is now auth.py (+2/-1) + test_config_and_middleware.py (+41). Local: TestRateLimitClientId 5 passed, ruff clean. Running codex + pr_validate; will squash-merge on green.

Thank you for stepping in to refine the scope and correct the TYPE_CHECKING gate issue. I appreciate the clear feedback and the merge.